Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical IPC3 build failures and audio-buffer and format-handling issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a Steam Audio spatial-audio offload component, DSP processing, IPC4 controls, BVH helpers, PTL integration, and LLEXT packaging.
Changes:
- Registers the component UUID and PTL module configuration.
- Adds spatial-audio, reverb, metadata, and BVH processing.
- Integrates static/LLEXT builds through Kconfig and CMake.
File summaries
| File | Reviewed changes and findings |
|---|---|
uuid-registry.txt |
Registers the Steam Audio component UUID. |
tools/rimage/config/ptl.toml.h |
Adds the module to PTL image configuration. |
src/audio/steamaudio/steamaudio.toml |
Defines module capabilities. critical (3 votes): Output advertises all channel configurations, but processing only writes stereo; restrict output or implement channel-aware processing. |
src/audio/steamaudio/steamaudio.h |
Defines protocol, state, and APIs. moderate (1 vote): Documented SMTA magic differs from the STEA constant used by the component. nit (2 votes): Add Doxygen parameter and return documentation. |
src/audio/steamaudio/steamaudio.c |
Implements lifecycle and adapter registration. moderate (1 vote): Discards source_to_sink_copy() errors. moderate (1 vote): Reset does not clear delay buffers or direct filter state. |
src/audio/steamaudio/steamaudio-ipc4.c |
Implements IPC4 controls. moderate (3 votes): Directivity, air absorption, and transmission are accepted but unused. moderate (3 votes): Reverb fields other than wet gain are unused. moderate (1 vote): BVH query control is unhandled. moderate (3 votes): Getter does not update data_offset_size. moderate (3 votes): Ambisonics configuration is stored but unused. moderate (1 vote): Direct getter leaves response fields uninitialized. moderate (1 vote): Binaural getter omits interpolation and HRTF slot fields. |
src/audio/steamaudio/steamaudio-generic.c |
Implements audio processing. critical (3 votes): Circular-buffer pointers are processed as contiguous arrays, risking overrun at wrap boundaries. moderate (1 vote): S16 source lock is not released when sink acquisition fails. moderate (3 votes): Only channel 0 is processed. critical (3 votes): PCM data is interpreted as metadata without dedicated-stream and length validation. moderate (2 votes): FDN delay buffers are not cleared on reset. moderate (3 votes): Elevation and HRIR data are unused. moderate (1 vote): S32 source lock is not released on sink failure. moderate (1 vote): S32 processing also drops all but channel 0. moderate (1 vote): Room geometry is not used for tracing or occlusion. moderate (1 vote): S24 is incorrectly routed through S32 conversion. |
src/audio/steamaudio/steamaudio_bvh.c |
Adds BVH helpers. moderate (1 vote): The single-node BVH has invalid child indices and should either build valid nodes or remain explicitly linear. |
src/audio/steamaudio/llext/llext.toml.h |
Defines the LLEXT manifest configuration. |
src/audio/steamaudio/llext/CMakeLists.txt |
Builds the LLEXT module. critical (1 vote): IPC4-only sources are compiled for IPC3 LLEXT configurations. |
src/audio/steamaudio/Kconfig |
Adds component configuration. critical (2 votes): The component is enabled for every IPC variant although IPC3 lacks the referenced configuration symbols. |
src/audio/steamaudio/CMakeLists.txt |
Integrates component sources. critical (2 votes): IPC3 builds omit the IPC4 implementation while the interface still references its symbols. |
src/audio/Kconfig |
Registers the component Kconfig. |
src/audio/CMakeLists.txt |
Adds the component directory to the build. |
app/boards/intel_adsp_ace30_ptl.conf |
Enables PTL LLEXT configuration. |
Review details
Suppressed comments (14)
src/audio/steamaudio/steamaudio-generic.c:330
- The source read lock is still held when
sink_get_buffer_s16()fails. Becausesource_get_data_s16()records an outstanding request, the next process call will return-EBUSYand can stall the pipeline. Release the source fragment with a zero free size before returning the error.
if (ret)
return ret;
src/audio/steamaudio/steamaudio-generic.c:377
- The 32-bit path obtains circular source/sink fragments but then treats both pointers as contiguous arrays. A wrapped ring position will read or write outside the corresponding buffer instead of processing all requested samples. Handle the two circular segments using the returned buffer starts/sizes.
ret = source_get_data_s32(source, in_bytes, &src, &x_start, &x_size);
if (ret)
return ret;
ret = sink_get_buffer_s32(sink, out_bytes, &dst, &y_start, &y_size);
src/audio/steamaudio/steamaudio-generic.c:379
- The source read lock is still held when
sink_get_buffer_s32()fails. Sincesource_get_data_s32()records an outstanding request, the next process call can return-EBUSYand stall the pipeline. Release the source fragment with a zero free size before returning the error.
if (ret)
return ret;
src/audio/steamaudio/steamaudio-generic.c:384
- The S32 implementation also discards every input channel except channel 0, so multichannel sources lose voices even when the module is configured for them. Keep the channel handling consistent with the declared input contract rather than silently ignoring
cd->channels - 1channels.
for (uint32_t i = 0; i < frames; i++)
cd->in_scratch[i] = (float)src[i * cd->channels] * (1.0f / 2147483648.0f);
src/audio/steamaudio/steamaudio-generic.c:169
- The room scene is initialized but no processing path calls
steamaudio_dsp_trace_closest_hit()orsteamaudio_dsp_test_occlusion(). Consequently this geometry cannot affect the audio, so the advertised DSP BVH/room occlusion rendering is not active.
/* Initialize BVH scene with standard test room (8m x 10m x 3.5m) */
steamaudio_dsp_scene_init_box_room(&cd->scene, 8.0f, 10.0f, 3.5f);
src/audio/steamaudio/steamaudio-generic.c:300
- A malformed synchronized header with
num_samples == 0reaches this division and produces an invalid gain step. Validate the header fields, including a nonzero sample count, before using them.
cd->direct.gain_step = (cd->direct.target_gain - cd->direct.current_gain) / (float)hdr->num_samples;
src/audio/steamaudio/steamaudio-generic.c:414
- This S24 path is routed to the S32 handler, which converts input with Q1.31 scaling and emits Q1.31 output. SOF S24_4LE is Q1.23; other processors sign-extend it and shift by 8, so this path attenuates S24 input by roughly 256 and writes an invalid S24 representation. Use a format-specific conversion or pass the source format into the handler.
case SOF_IPC_FRAME_S24_4LE:
return steamaudio_process_s32;
src/audio/steamaudio/steamaudio-ipc4.c:106
STEAMAUDIO_PARAM_BVH_QUERYis defined as a public control but falls through to the unhandled-parameter error. The host cannot provide a scene or query through this API, and the renderer therefore remains tied to the fixed test room initialized insteamaudio_dsp_init(). Implement the control or remove the advertised interface.
default:
comp_warn(dev, "steamaudio: unhandled param_id 0x%x", param_id);
return -EINVAL;
}
src/audio/steamaudio/steamaudio-ipc4.c:125
- The direct getter writes only
comp_type, distance, and occlusion; the remaining fields in the packed response (flags, transmission type, directivity, and both EQ arrays) are left as whatever was in the caller's buffer. Populate the full state or at least clear unsupported fields before returning a configuration.
struct sof_steamaudio_direct_config *cfg = (struct sof_steamaudio_direct_config *)fragment;
cfg->comp_type = STEAMAUDIO_PARAM_DIRECT_CONFIG;
cfg->distance_attenuation = cd->direct.current_gain;
cfg->occlusion = 0.0f;
src/audio/steamaudio/steamaudio-ipc4.c:138
- The binaural getter similarly omits
interpolationandhrtf_slot_id, leaving those response fields uninitialized. Clear the response and fill every field, or reject/omit a getter that cannot represent the current configuration.
cfg->comp_type = STEAMAUDIO_PARAM_BINAURAL_CONFIG;
cfg->direction[0] = cd->binaural.direction[0];
cfg->direction[1] = cd->binaural.direction[1];
cfg->direction[2] = cd->binaural.direction[2];
cfg->spatial_blend = cd->binaural.spatial_blend;
src/audio/steamaudio/steamaudio.c:60
- The return value is discarded here.
source_to_sink_copy()can return-EFBIGor-ENOSPCwithout consuming/committing data, but returning zero tells the adapter that processing succeeded and can leave the pipeline stalled. Return the copy result.
source_to_sink_copy(source, sink, true, frames * cd->frame_bytes);
src/audio/steamaudio/steamaudio.c:103
- The reset path calls steamaudio_dsp_init, but that function resets reverb indices without clearing delay_buffers (and does not clear the direct filter states). After a pipeline reset, pre-reset reverb/filter history can leak into the new stream instead of starting from silence.
steamaudio_dsp_init(cd, cd->sample_rate);
src/audio/steamaudio/steamaudio.h:19
- The PR describes the compressed metadata protocol as SMTA (0x534D5441), but this component defines and checks STEA (0x53544541). Packets produced with the documented SMTA magic will be ignored by check_inband_bitstream; the producer and DSP must use one protocol constant.
#define STEAMAUDIO_SOF_SYNC_WORD 0x53544541 /* 'STEA' */
#define STEAMAUDIO_SOF_PROTOCOL_VERSION 0x00010000
src/audio/steamaudio/steamaudio_bvh.c:177
- Only one BVH node is created, but its child indices are set to 0 and 11 even though no child nodes exist;
steamaudio_dsp_trace_closest_hit()then ignores these fields and linearly scans every triangle. This is not a valid BVH, and a traversal using these indices would access outsidenodes. Build valid child nodes or keep the scene representation explicitly linear.
scene->num_nodes = 1;
scene->nodes[0].bounds.min = (struct dsp_vec3){ x0, y0, z0 };
scene->nodes[0].bounds.max = (struct dsp_vec3){ x1, y1, z1 };
scene->nodes[0].left_child = 0;
scene->nodes[0].right_child = 11;
- Files reviewed: 15/15 changed files
- Comments generated: 14
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+11
to
+13
| if(CONFIG_IPC_MAJOR_4) | ||
| add_local_sources(sof steamaudio-ipc4.c) | ||
| endif() |
Comment on lines
+3
to
+5
| config COMP_STEAMAUDIO | ||
| tristate "Steam Audio spatial offload component" | ||
| default y |
| SOURCES ../steamaudio.c | ||
| ../steamaudio-generic.c | ||
| ../steamaudio_bvh.c | ||
| ../steamaudio-ipc4.c |
Comment on lines
+324
to
+328
| ret = source_get_data_s16(source, in_bytes, &src, &x_start, &x_size); | ||
| if (ret) | ||
| return ret; | ||
|
|
||
| ret = sink_get_buffer_s16(sink, out_bytes, &dst, &y_start, &y_size); |
| if (ret) | ||
| return ret; | ||
|
|
||
| check_inband_bitstream(cd, src, in_bytes); |
Comment on lines
+46
to
+47
| cd->direct.target_gain = cfg->distance_attenuation * (1.0f - cfg->occlusion); | ||
| cd->direct.gain_step = (cd->direct.target_gain - cd->direct.current_gain) / 128.0f; |
Comment on lines
+77
to
+78
| cd->reverb.wet_gain = cfg->wet_gain; | ||
| comp_dbg(dev, "steamaudio: reverb wet=%f", (double)cfg->wet_gain); |
Comment on lines
+89
to
+90
| cd->ambisonics.order = cfg->order; | ||
| memcpy(cd->ambisonics.rotation, cfg->listener_rotation, sizeof(cd->ambisonics.rotation)); |
| cfg->comp_type = STEAMAUDIO_PARAM_DIRECT_CONFIG; | ||
| cfg->distance_attenuation = cd->direct.current_gain; | ||
| cfg->occlusion = 0.0f; | ||
| return 0; |
Comment on lines
+212
to
+214
| /* Public component API */ | ||
| steamaudio_func steamaudio_find_proc_func(enum sof_ipc_frame src_fmt); | ||
| void steamaudio_dsp_init(struct steamaudio_comp_data *cd, uint32_t sample_rate); |
lgirdwood
force-pushed
the
topic/steamaudio-offload
branch
from
September 15, 2026 14:42
240ae28 to
53b43b6
Compare
Member
Author
|
Addressed all automated review comments in commits
Also extended the DSP engine with Multi-Channel Surround Panning (VBAP Stereo/Quad/5.1/7.1), Virtual Surround Sound downmixing, and Higher-Order Ambisonics (HOA Orders 1-3). |
Member
Author
|
Added Phase 7: Acoustic Pathing & Knife-Edge Diffraction Offload + Steam Audio Host C API Integration (
|
lgirdwood
force-pushed
the
topic/steamaudio-offload
branch
from
October 4, 2026 18:33
5ce3506 to
798ebbe
Compare
Add global UUID registration for Steam Audio (53746561-6d61-7564-696f737465616d31), declare sys_comp_module_steamaudio_interface_init() in component.h, and include steamaudio.toml in rimage manifest configurations across PTL, MTL, and LNL platforms for modular LLEXT firmware packaging. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Define the protocol interface, IPC4 control parameters, and mathematical DSP helpers for the Steam Audio spatial audio offload component: - Component types (0x1001-0x1033) for direct, binaural, ambisonics, reverb, panning, virtual surround, pathing, LOD, and scene upmix. - Speaker layout definitions (Stereo, Quad, 5.1, 7.1) and output modes. - 128-byte aligned kcontrol structures for voice LOD and scene upmixing. - Fast vector, matrix, quaternion, and SIMD transcendental approximations. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…nfrastructure Implement the SOF component adapter lifecycle hooks and IPC4 control dispatchers for Steam Audio: - Module interface: steamaudio_init, prepare, process, reset, free. - IPC4 set/get configuration handlers with strict bounds checking, payload size validation, and kcontrol state management. - Zero-copy mmap/DMA ring buffer synchronization for dynamic geometry and voice metadata streaming. - Build system integration supporting both built-in firmware builds and dynamically loadable LLEXT ELF modules. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…lation engines Implement the core spatial audio signal processing engines: - Direct path sound propagation: distance attenuation, ISO 9613-1 air absorption, delay lines, and frequency-dependent directivity patterns. - Binaural audio: 128-tap time-domain HRIR direct convolution and near-field parallax / proximity effect filter. - Ambisonics: Orders 1, 2, 3 Higher-Order Ambisonics (HOA) encoding (4, 9, 16 channels) and 3D rotational transformations. - Late reverberation: 8-line Feedback Delay Network (FDN) with Householder diffusion matrix and band-limited damping filters. - Surround panning: 2D pairwise constant-power panning law for 2.0, 4.0, 5.1, and 7.1 speaker layouts. - Virtual surround: Psychoacoustic ITD and ILD cues for headphones. - Environmental acoustics: atmospheric turbulence, surface acoustic scattering, sound barrier edge diffraction, and Doppler shift. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
… acoustic upmixer Implement on-DSP geometric acoustics, autonomous voice scheduling, and scene-aware multi-channel acoustic upmixing: - Bounding Volume Hierarchy (BVH): Instanced triangle mesh scene graph with Möller-Trumbore ray casting for real-time occlusion and acoustic material transmission. - Acoustic Probe Graph: Dijkstra shortest-path acoustic search across clustered spatial reflection probes. - 3-Tier Dynamic Voice LOD Governor: Autonomous on-DSP cycle governor scheduling Tier 1 (HRTF), Tier 2 (HOA), and Tier 3 (Diffuse) voices with dynamic voice culling and crossfades. - Scene-Aware 5.1/7.1 Acoustic Upmixer: Mid/Side dialogue anchoring on center channel, lateral all-pass Schroeder decorrelation delays, Householder surround reverb diffusion, and 4th-order Linkwitz-Riley (LR4) 80 Hz bass management crossover. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…eamaudio Add mathematical verification unit tests executable on host and CI testbench: - Test 1: Constant-power panning law verification across 360-degree azimuths for Stereo, Quad, 5.1, and 7.1 layouts (sum(w_i^2) == 1.0). - Test 2: Virtual surround psychoacoustic ITD and ILD acoustic cues. - Test 3: Higher-Order Ambisonics Orders 1, 2, and 3 spherical harmonics basis orthogonality and energy conservation. - Test 4: Bit-exact S24_4LE Q1.23 sign-extension and full-scale conversion. - Test 5: Möller-Trumbore BVH occlusion ray tracing on box-room geometry. - Register IPC4 component interface hook in utils_ipc4.c. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add an independent ALSA Topology v2 Feature Topology
(sof-steamaudio-feature.tplg):
- Define Class.Widget."steamaudio" with UUID, volume/switch mixer
controls, and 4096-byte configuration bytes control.
- Define Class.Pipeline."steamaudio-playback".
- Declare 64-channel PCM 60 ("SteamAudio Spatial Playback", 48kHz
S16/S24/S32) and compressed metadata PCM 61 ("SteamAudio Metadata",
compress "true").
- Route dynamically into base topology render paths via
$STEAMAUDIO_SINK_MIXOUT (default "mixout.1.1").
- Add CMake build targets in tplg-targets-ace1.cmake and
tplg-targets-ace3.cmake.
- Enables modular feature topology overlay via
snd_sof.feature_topologies=sof-steamaudio-feature.tplg without
modifying base topology files.
Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
…guide Add comprehensive documentation for Steam Audio spatial audio offload: - Architectural block diagram covering dual-ring buffer IPC4 model, host-to-DSP shared memory layout, and DSP execution stages. - Memory footprint requirements, cycle budgets, and SIMD optimization. - End-to-end latency benchmarks (0.916 ms @ 48kHz / 1ms period) and game frame rate scaling charts (30 Hz, 60 Hz, 120 Hz, 144 Hz). - ALSA Topology v2 integration guide and runbook for testing with alsatplg and sof-testbench. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
lgirdwood
force-pushed
the
topic/steamaudio-offload
branch
from
October 7, 2026 20:12
5f47be7 to
450584d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request adds an offload module and dynamically loadable ELF extension (LLEXT) for Valve Steam Audio spatial audio rendering engine in Sound Open Firmware (SOF).
Aim is to provide
This work will not help increase frame rate where games are GPU bound, but will save some cycles from CPUs that can be then scheduled for render submission threads. This may result in a small FPS increase for non GPU bound games.
Target platforms: Intel ACE 3.0 (Panther Lake / PTL) and Intel ACE 1.5 (Arrow Lake / ARL-S) Xtensa DSPs.
Architecture Highlights
active_sources_mask) within the audio processing loop, avoiding unnecessary processing overhead.snd_compr):snd_compr) using magic protocol0x53544541(STEA).steamaudio.llext) usingCONFIG_COMP_STEAMAUDIO=m.53746561-6d61-7564-696f737465616d31inuuid-registry.txt.STEAMAUDIO_PARAM_OUTPUT_MODE.src/audio/steamaudio/README.md.Real-World Game Benchmarks (Dota 2 & Counter-Strike 2)
These are limited test runs showing no overall harm from offloading, with improved frame time latency on c2s/dota. Full evalaation with multiple titles and platforms TBD. Evaluated on ** DUT (Intel Arrow Lake / ACE 1.5 DSP)** running native Linux Steam titles with Valve Steam Audio (
libsteamaudio.so/libphonon.so):Measurement Methodology
timedemoengine running pro tournament replay8997682537.demacross 2,999 identical simulation ticks (ticks 1000 to 4000), analyzed viaSource2BenchV2.csvandtimedemo_profile.csv.VK_LAYER_MANGOHUD_overlay_x86_64) synchronized 1s after[Server] BeginMatchonde_dust2over 25 seconds of active firefights.pidstat -t -p <PID>isolating spatial audio threads (CSteamAudioReve,CSteamAudioPart,AudioMixer) and 16 concurrentAsync P+worker threads./sys/class/powercap/intel-rapl/intel-rapl:0/energy_ujatTest Machine Configuration
powersavegovernor).7.3.0-rc1-sof-dev+.sof-arl-hdmi.tplg, PipeWirepro-audioprofile endpoint 800 Series ACE Pro (HDMI 1, PCM 3).SteamLinuxRuntime_sniperunder Vulkan 1.3.Signed-off-by: Liam Girdwood liam.r.girdwood@linux.intel.com